Skip to content

Maintenance: Refactor AI filter code#58

Merged
turt2live merged 11 commits into
mainfrom
travis/ai
Jan 2, 2026
Merged

Maintenance: Refactor AI filter code#58
turt2live merged 11 commits into
mainfrom
travis/ai

Conversation

@turt2live

@turt2live turt2live commented Dec 23, 2025

Copy link
Copy Markdown
Member

Reviewable commit-by-commit (recommended)

I wasn't overly happy with the original code, but it worked, so it was fine. Now that we're looking at adding other models/providers/classifiers it feels like a good idea to make the code a bit cleaner.

Previously, the code abstracted the idea of a provider within a single generic "OpenAI filter". This meant it was testable, but was complicated to set up and follow the code path. The abstraction was also at the wrong layer: it should have been at a higher level, away from the filter itself, so it could (theoretically) be reused elsewhere, possibly in the future. The old code could theoretically be abused to be reusable, like in this PR, but the result is a bit messy and screams for wanting to abstract away the AIProvider stuff to a higher level.

This PR moves the abstraction to that higher level, promoting models (what we call "AI providers") to a first-class citizen in our code. This PR also splits the AI filter to have a shareable InstancedFilter for future model support with overly specific non-instanced filters.

Because of the new layering and new OpenAI library, we're also able to mock the OpenAI layer and test that our code handles responses in a reasonably safe way. Previously, we'd have to just try it out or wait for something to happen.

This refactoring also moves the filter.Media struct to media.Item because that's probably where it should have been at first. The event rendering code is also moved to a dedicated package for reusability. Both moves are to prevent import cycles (filter -> ai -> filter).

All of this means that when we go to add a second model, we'd:

  1. Create a new provider in the ai package (with appropriate test)
  2. Copy/paste filter_openai_omni to create a new non-instanced filter
  3. Ensure the filter gets used in the community manager

... and that's it, hopefully. This then takes care of per-room enablement, vendor lock-in avoidance, etc for us. (Vendor lock-in is avoided by simply adding more and more models with minimal dependencies or copy/paste.)

Pull Request Checklist

  • Pull request is based on the main branch.
  • Pull request title describes the changes in a way a user would understand. For example, "Fix keywords filter not applying" instead of "Read keywords slice in a loop".
  • Code style matches surrounding code.
  • Tests are added for new code (and existing code) if possible.
    • If not possible, please explain why in your PR description.
  • The CI checks pass.

@turt2live turt2live marked this pull request as ready for review December 23, 2025 03:25
@turt2live turt2live requested a review from a team as a code owner December 23, 2025 03:25
Comment thread ai/openai_omni_moderation_test.go Outdated

// We create our own HTTP client to intercept and act as the OpenAI API
apiKey := "not_a_real_key"
mockApi := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am vaguely wondering if putting the mock api code in a separate file makes sense, as it might a little easier/cleaner to read?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably, yea. I'll take a look and see what I can do.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split it out to the test package - please take a look

Comment thread filter/filter_openai_omni_test.go Outdated
RawJSON() string // same definition that's shared with the OpenAI response parts
}

func compressJsonResponse(target compressible) string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this removes duplicate keys and newlines?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in a way, yea. For whatever reason the OpenAI response structs duplicate all of their fields and then use "pretty" JSON too, which makes things noisy.

turt2live and others added 2 commits December 23, 2025 15:43
Co-authored-by: Shay <hillerys@element.io>
... and split it out a bit further for readability.
@turt2live turt2live requested a review from H-Shay December 27, 2025 10:57

@H-Shay H-Shay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great thanks for splitting that out - much clearer!

@turt2live turt2live merged commit 6af3df3 into main Jan 2, 2026
3 checks passed
@turt2live turt2live deleted the travis/ai branch January 2, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants